✨ Tsx #66
✨ Tsx #66
Conversation
WalkthroughThis pull request integrates the new package Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant TSX as @ast-grep/lang-tsx
participant NAPI as @ast-grep/napi
participant Nursery as @ast-grep/nursery
Dev->>TSX: Install and import package
TSX->>NAPI: Register dynamic TSX language (registerDynamicLanguage)
Dev->>NAPI: Call parse('tsx', code) to parse TSX code
NAPI-->>Dev: Return syntax tree (including root node)
Nursery->>NAPI: Execute test runner on TSX code
NAPI-->>Nursery: Provide parsed node for assertion
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/tsx/nursery.js (1)
12-18: Consider enhancing the test coverage.The test runner successfully validates that the parser can parse a JSX element and correctly identify its node kind. However, it could be strengthened with additional assertions.
testRunner: parse => { const sg = parse('<Button onClick={() => foo()}>Click me</Button>') const root = sg.root() const node = root.find('<$ELEMENT onClick={$HANDLER}>$$$</$ELEMENT>') assert.equal(node.kind(), 'jsx_element') + // Verify the element name + const elementName = node.find('$ELEMENT') + assert.equal(elementName.text(), 'Button') + // Verify the handler exists + assert.ok(node.find('$HANDLER').text().includes('foo()')) },packages/tsx/README.md (2)
1-2: Improve the package description.The title is concise but could provide more context about what ast-grep is and the purpose of this language package.
-# ast-grep napi language for tsx +# ast-grep napi language for TSX + +A TypeScript JSX (TSX) language package for ast-grep, enabling precise pattern matching and manipulation of TSX abstract syntax trees.
14-24: Provide a more realistic code example.The usage example is clear but uses a placeholder for the code string. A real TSX example would be more helpful to users.
registerDynamicLanguage({ tsx }) -const sg = parse('tsx', `your code`) +const sg = parse('tsx', ` + function App() { + return ( + <div className="app"> + <Button onClick={() => console.log('clicked')}> + Click me + </Button> + </div> + ); + } +`) sg.root().kind()packages/tsx/package.json (1)
21-23: Complete the package metadata.The author field is empty, and there are other standard metadata fields missing.
"keywords": ["ast-grep"], - "author": "", + "author": "The ast-grep contributors", "license": "ISC", + "repository": { + "type": "git", + "url": "https://github.com/ast-grep/ast-grep" + }, + "homepage": "https://github.com/ast-grep/ast-grep/tree/main/packages/tsx",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/cyan-crabs-relax.md(1 hunks)packages/tsx/README.md(1 hunks)packages/tsx/index.d.ts(1 hunks)packages/tsx/index.js(1 hunks)packages/tsx/nursery.js(1 hunks)packages/tsx/package.json(1 hunks)packages/tsx/postinstall.js(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/tsx/nursery.js (2)
packages/tsx/index.js (1)
path(1-1)scripts/nursery/index.ts (1)
setup(38-46)
🔇 Additional comments (12)
.changeset/cyan-crabs-relax.md (1)
1-6: LGTM: Changeset correctly configured for the new TSX package.The changeset is correctly formatted to add
@ast-grep/lang-tsxas a patch release with a clear description.packages/tsx/postinstall.js (1)
1-4: LGTM: Postinstall script is properly configured.The postinstall script correctly imports the setup function and provides the necessary configuration. This follows the pattern used in other language packages.
packages/tsx/index.d.ts (2)
1-7: LGTM: Type definition for LanguageRegistration is well-structured.The type definition correctly defines the structure needed for language registration with appropriate optional properties.
9-10: LGTM: Registration constant and export are properly defined.The constant is correctly typed and exported as the default export.
packages/tsx/index.js (2)
1-2: LGTM: Parser path resolution is correct.The code properly uses Node's path module to resolve the location of the parser library.
4-9:✅ Verification successful
LGTM: Module exports are properly configured for TSX support.
The module exports all necessary configuration properties for the TSX language support. The extension, language symbol, and expando character are correctly specified.
🏁 Script executed:
#!/bin/bash # Verify that parser.so will be available after installation # Check if there's any mechanism to build or download parser.so # This could be in package.json scripts or the setup-lang package echo "Checking for parser.so generation mechanism..." cat packages/tsx/package.json | grep -E "\"(pre|post)?(install|build)\"" find packages/tsx -type f -name "*.js" | xargs grep -l "parser.so" # Check if setup-lang package downloads or builds the parser echo "Checking setup-lang package for parser handling..." if [ -d "packages/setup-lang" ]; then grep -r "parser.so" packages/setup-lang fiLength of output: 539
TSX configuration verified and LGTM
The build script in
packages/tsx/package.json("build": "tree-sitter build -o parser.so ./node_modules/tree-sitter-typescript/tsx") confirms thatparser.sowill be generated during the build process. The module exports inpackages/tsx/index.jscorrectly configure TSX support with the appropriate extension, language symbol, and expando character.packages/tsx/nursery.js (2)
1-5: Imports look good and follow the standard pattern.The necessary dependencies are correctly imported for the nursery setup, including the core nursery package, assertion module, and path utilities.
6-11: Setup configuration is properly defined.The configuration parameters match the expected format for the nursery setup, with clear specification of the language name, tree-sitter package, and source path.
packages/tsx/README.md (1)
3-12: Installation instructions are clear and complete.The installation section properly explains how to install the necessary packages, including the optional tree-sitter CLI for cases where no prebuilt version is available.
packages/tsx/package.json (3)
6-12: Scripts look well-defined and follow expected patterns.The scripts for building, testing, and publishing are properly configured. The use of nursery for source generation and testing is consistent with the package's purpose.
24-46: Dependencies and configuration are properly structured.The dependency structure correctly handles required packages and makes
tree-sitter-clian optional peer dependency. The publishing configuration and pnpm build settings are also appropriate.
13-20:❓ Verification inconclusive
File inclusion list is comprehensive.
All necessary files are included in the package distribution. Make sure the
postinstall.jsfile exists as it's listed here but wasn't included in the files for review.
🏁 Script executed:
#!/bin/bash # Check if postinstall.js exists in the package fd -t f "postinstall.js" --path-regex "packages/tsx/"Length of output: 291
Ensure the
postinstall.jsfile exists in the package directoryThe file inclusion list in
packages/tsx/package.json(lines 13–20) looks comprehensive; however, we need to verify that thepostinstall.jsfile—which is listed—is actually present. Note that the initial verification script failed due to an unrecognized--path-regexflag. Please either run a corrected check, for example:fd -t f "postinstall.js" packages/tsxor manually verify that
packages/tsx/postinstall.jsexists.
| { | ||
| "name": "@ast-grep/lang-tsx", | ||
| "version": "0.0.1", | ||
| "description": "", | ||
| "main": "index.js", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a meaningful package description.
The description field is currently empty, which isn't helpful for users discovering this package.
{
"name": "@ast-grep/lang-tsx",
"version": "0.0.1",
- "description": "",
+ "description": "TypeScript JSX (TSX) language support for ast-grep",
"main": "index.js",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "name": "@ast-grep/lang-tsx", | |
| "version": "0.0.1", | |
| "description": "", | |
| "main": "index.js", | |
| { | |
| "name": "@ast-grep/lang-tsx", | |
| "version": "0.0.1", | |
| "description": "TypeScript JSX (TSX) language support for ast-grep", | |
| "main": "index.js", |
2 participants
Summary by CodeRabbit